From bfc151949086a33d8f87ac36c7ae38de73348eb5 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Sat, 2 Aug 2014 00:08:31 -0700 Subject: [PATCH] Make freshness tests more robust with time travel Add a helper function to forcibly move a tree back one hour so new edits are picked up correctly. --- configure | 2 +- src/cargo/ops/cargo_generate_lockfile.rs | 9 ++++++-- src/cargo/ops/cargo_rustc/fingerprint.rs | 14 +++++++++++-- tests/support/mod.rs | 10 ++++++++- tests/support/paths.rs | 26 ++++++++++++++++++++++++ tests/test_cargo_compile_git_deps.rs | 2 ++ tests/test_cargo_compile_path_deps.rs | 10 +++++---- tests/test_cargo_doc.rs | 3 ++- tests/test_cargo_freshness.rs | 11 +++++----- tests/test_cargo_test.rs | 3 +++ tests/tests.rs | 1 - 11 files changed, 74 insertions(+), 17 deletions(-) diff --git a/configure b/configure index 34cd7ed38..091c04000 100755 --- a/configure +++ b/configure @@ -310,7 +310,7 @@ if [ $HELP -eq 0 ]; then else probe_need CFG_RUSTC rustc fi - DEFAULT_BUILD=$(${CFG_RUSTC} --version verbose | grep 'host: ' | sed 's/host: //') + DEFAULT_BUILD=$("${CFG_RUSTC}" --version verbose | grep 'host: ' | sed 's/host: //') fi valopt build "${DEFAULT_BUILD}" "GNUs ./configure syntax LLVM build triple" diff --git a/src/cargo/ops/cargo_generate_lockfile.rs b/src/cargo/ops/cargo_generate_lockfile.rs index 0c43a1d35..7661bf60e 100644 --- a/src/cargo/ops/cargo_generate_lockfile.rs +++ b/src/cargo/ops/cargo_generate_lockfile.rs @@ -1,6 +1,6 @@ #![warn(warnings)] use std::collections::HashSet; -use std::io::File; +use std::io::{mod, File}; use serialize::{Encodable, Decodable}; use toml::Encoder; @@ -138,7 +138,12 @@ pub fn write_resolve(pkg: &Package, resolve: &Resolve) -> CargoResult<()> { } let loc = pkg.get_root().join("Cargo.lock"); - try!(File::create(&loc).write_str(out.as_slice())); + let mut f = try!(File::open_mode(&loc, io::Open, io::ReadWrite)); + let prev = try!(f.read_to_string()); + if prev != out { + try!(f.seek(0, io::SeekSet)); + try!(f.write_str(out.as_slice())); + } Ok(()) } diff --git a/src/cargo/ops/cargo_rustc/fingerprint.rs b/src/cargo/ops/cargo_rustc/fingerprint.rs index ef0115a7c..2acf5c082 100644 --- a/src/cargo/ops/cargo_rustc/fingerprint.rs +++ b/src/cargo/ops/cargo_rustc/fingerprint.rs @@ -49,6 +49,8 @@ pub fn prepare_target(cx: &mut Context, pkg: &Package, target: &Target, let new_loc = new.join(filename.as_slice()); let doc = target.get_profile().is_doc(); + debug!("fingerprint at: {}", new_loc.display()); + // First bit of the freshness calculation, whether the dep-info file // indicates that the target is fresh. let (old_dep_info, new_dep_info) = dep_info_loc(cx, pkg, target, kind); @@ -109,6 +111,8 @@ pub fn prepare_build_cmd(cx: &mut Context, pkg: &Package) let old_loc = old.join("build"); let new_loc = new.join("build"); + debug!("fingerprint at: {}", new_loc.display()); + let new_fingerprint = try!(calculate_build_cmd_fingerprint(cx, pkg)); let new_fingerprint = mk_fingerprint(cx, &new_fingerprint); @@ -195,16 +199,22 @@ fn calculate_target_fresh(pkg: &Package, dep_info: &Path) -> CargoResult { Some(Ok(line)) => line, _ => return Ok(false), }; + let line = line.as_slice(); let mtime = try!(fs::stat(dep_info)).modified; - let deps = try!(line.as_slice().splitn(':', 1).skip(1).next().require(|| { + let pos = try!(line.find_str(": ").require(|| { internal(format!("dep-info not in an understood format: {}", dep_info.display())) })); + let deps = line.slice_from(pos + 2); for file in deps.split(' ').map(|s| s.trim()).filter(|s| !s.is_empty()) { match fs::stat(&pkg.get_root().join(file)) { Ok(stat) if stat.modified <= mtime => {} - _ => { debug!("stale: {}", file); return Ok(false) } + Ok(stat) => { + debug!("stale: {} -- {} vs {}", file, stat.modified, mtime); + return Ok(false) + } + _ => { debug!("stale: {} -- missing", file); return Ok(false) } } } diff --git a/tests/support/mod.rs b/tests/support/mod.rs index 07060671c..4de930804 100644 --- a/tests/support/mod.rs +++ b/tests/support/mod.rs @@ -386,7 +386,15 @@ impl ham::Matcher for Execs { Err(ProcessError { output: Some(ref out), .. }) => { self.match_output(out) } - Err(e) => Err(format!("could not exec process {}: {}", process, e)) + Err(e) => { + let mut s = format!("could not exec process {}: {}", process, e); + match e.cause { + Some(cause) => s.push_str(format!("\ncaused by: {}", + cause).as_slice()), + None => {} + } + Err(s) + } } } } diff --git a/tests/support/paths.rs b/tests/support/paths.rs index 5dc6c86c3..f53cc02d0 100644 --- a/tests/support/paths.rs +++ b/tests/support/paths.rs @@ -26,6 +26,7 @@ pub fn home() -> Path { pub trait PathExt { fn rm_rf(&self) -> IoResult<()>; fn mkdir_p(&self) -> IoResult<()>; + fn move_into_the_past(&self) -> IoResult<()>; } impl PathExt for Path { @@ -58,6 +59,31 @@ impl PathExt for Path { fn mkdir_p(&self) -> IoResult<()> { fs::mkdir_recursive(self, io::UserDir) } + + fn move_into_the_past(&self) -> IoResult<()> { + if self.is_file() { + try!(time_travel(self)); + } else { + for f in try!(fs::walk_dir(self)) { + if !f.is_file() { continue } + try!(time_travel(&f)); + } + } + return Ok(()); + + fn time_travel(path: &Path) -> IoResult<()> { + let stat = try!(path.stat()); + + let hour = 1000 * 3600; + let mut newtime = stat.modified - hour; + // FIXME: this looks like a bug on windows that needs to be fixed + // upstream + if cfg!(windows) { + newtime = newtime * 1000; + } + fs::change_file_times(path, newtime, newtime) + } + } } /// Ensure required test directories exist and are empty diff --git a/tests/test_cargo_compile_git_deps.rs b/tests/test_cargo_compile_git_deps.rs index 6b4b9ea0d..3afbfe366 100644 --- a/tests/test_cargo_compile_git_deps.rs +++ b/tests/test_cargo_compile_git_deps.rs @@ -3,6 +3,7 @@ use std::io::File; use support::{ProjectBuilder, ResultTest, project, execs, main_file, paths}; use support::{cargo_dir}; use support::{COMPILING, FRESH, UPDATING}; +use support::paths::PathExt; use hamcrest::{assert_that,existing_file}; use cargo; use cargo::util::{ProcessError, process}; @@ -567,6 +568,7 @@ test!(recompilation { {} foo v0.5.0 (file:{})\n", FRESH, git_project.root().display(), FRESH, p.root().display()))); + p.root().move_into_the_past().assert(); // Update the dependency and carry on! assert_that(p.process(cargo_dir().join("cargo-update")), diff --git a/tests/test_cargo_compile_path_deps.rs b/tests/test_cargo_compile_path_deps.rs index 194f10e52..be7ee8c83 100644 --- a/tests/test_cargo_compile_path_deps.rs +++ b/tests/test_cargo_compile_path_deps.rs @@ -1,8 +1,8 @@ use std::io::File; -use std::io::timer; use support::{ResultTest, project, execs, main_file, cargo_dir}; use support::{COMPILING, FRESH}; +use support::paths::PathExt; use hamcrest::{assert_that, existing_file}; use cargo; use cargo::util::{process}; @@ -231,6 +231,7 @@ test!(no_rebuild_dependency { {} foo v0.5.0 (file:{})\n", FRESH, bar.display(), FRESH, p.root().display()))); + p.root().move_into_the_past().assert(); p.build(); // rebuild the files (rewriting them in the process) assert_that(p.process(cargo_dir().join("cargo-build")), @@ -312,7 +313,7 @@ test!(deep_dependencies_trigger_rebuild { // // We base recompilation off mtime, so sleep for at least a second to ensure // that this write will change the mtime. - timer::sleep(1000); + p.root().move_into_the_past().assert(); File::create(&p.root().join("baz/src/baz.rs")).write_str(r#" pub fn baz() { println!("hello!"); } "#).assert(); @@ -325,6 +326,7 @@ test!(deep_dependencies_trigger_rebuild { COMPILING, p.root().display()))); // Make sure an update to bar doesn't trigger baz + p.root().move_into_the_past().assert(); File::create(&p.root().join("bar/src/bar.rs")).write_str(r#" extern crate baz; pub fn bar() { println!("hello!"); baz::baz(); } @@ -448,8 +450,8 @@ test!(nested_deps_recompile { {} foo v0.5.0 (file:{})\n", COMPILING, bar.display(), COMPILING, p.root().display()))); - // See comments for the above `sleep` - timer::sleep(1000); + p.root().move_into_the_past().assert(); + File::create(&p.root().join("src/foo.rs")).write_str(r#" fn main() {} "#).assert(); diff --git a/tests/test_cargo_doc.rs b/tests/test_cargo_doc.rs index c6fc2856e..cb89187f8 100644 --- a/tests/test_cargo_doc.rs +++ b/tests/test_cargo_doc.rs @@ -130,7 +130,8 @@ test!(doc_deps { assert_that(&p.root().join("target/doc/foo/index.html"), existing_file()); assert_that(&p.root().join("target/doc/bar/index.html"), existing_file()); - assert_that(p.process(cargo_dir().join("cargo-doc")), + assert_that(p.process(cargo_dir().join("cargo-doc")) + .env("RUST_LOG", Some("cargo::ops::cargo_rustc::fingerprint")), execs().with_status(0).with_stdout(format!("\ {fresh} bar v0.0.1 (file:{dir}) {fresh} foo v0.0.1 (file:{dir}) diff --git a/tests/test_cargo_freshness.rs b/tests/test_cargo_freshness.rs index 94bd19a05..54d1490f1 100644 --- a/tests/test_cargo_freshness.rs +++ b/tests/test_cargo_freshness.rs @@ -1,8 +1,8 @@ use std::io::{fs, File}; -use time; use support::{project, execs}; use support::{COMPILING, cargo_dir, ResultTest, FRESH}; +use support::paths::PathExt; use hamcrest::{assert_that, existing_file}; fn setup() {} @@ -29,6 +29,7 @@ test!(modifying_and_moving { execs().with_status(0).with_stdout(format!("\ {fresh} foo v0.0.1 (file:{dir}) ", fresh = FRESH, dir = p.root().display()))); + p.root().move_into_the_past().assert(); File::create(&p.root().join("src/a.rs")).write_str("fn main() {}").assert(); assert_that(p.process(cargo_dir().join("cargo-build")), @@ -67,19 +68,19 @@ test!(modify_only_some_files { assert_that(&p.bin("foo"), existing_file()); - let past = time::precise_time_ns() / 1_000_000 - 5000; - let lib = p.root().join("src/lib.rs"); let bin = p.root().join("src/b.rs"); let test = p.root().join("tests/test.rs"); File::create(&lib).write_str("invalid rust code").assert(); - fs::change_file_times(&lib, past, past).assert(); + lib.move_into_the_past().assert(); + p.root().move_into_the_past().assert(); File::create(&bin).write_str("fn foo() {}").assert(); // Make sure the binary is rebuilt, not the lib - assert_that(p.process(cargo_dir().join("cargo-build")), + assert_that(p.process(cargo_dir().join("cargo-build")) + .env("RUST_LOG", Some("cargo::ops::cargo_rustc::fingerprint")), execs().with_status(0).with_stdout(format!("\ {compiling} foo v0.0.1 (file:{dir}) ", compiling = COMPILING, dir = p.root().display()))); diff --git a/tests/test_cargo_test.rs b/tests/test_cargo_test.rs index e6509e958..56878e55e 100644 --- a/tests/test_cargo_test.rs +++ b/tests/test_cargo_test.rs @@ -3,6 +3,7 @@ use std::str; use support::{project, execs, basic_bin_manifest, basic_lib_manifest}; use support::{COMPILING, cargo_dir, ResultTest, FRESH}; +use support::paths::PathExt; use hamcrest::{assert_that, existing_file}; use cargo::util::process; @@ -552,6 +553,7 @@ test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured\n\n\ ", compiling = COMPILING, dir = p.root().display()).as_slice())); + p.root().move_into_the_past().assert(); assert_that(p.process(cargo_dir().join("cargo-test")), execs().with_status(0) .with_stdout(format!("\ @@ -592,6 +594,7 @@ test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured\n\n\ ", compiling = COMPILING, dir = p.root().display()).as_slice())); + assert_that(p.process(cargo_dir().join("cargo-test")), execs().with_status(0) .with_stdout(format!("\ diff --git a/tests/tests.rs b/tests/tests.rs index f86a631e5..67b886339 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -1,7 +1,6 @@ #![feature(macro_rules)] #![feature(phase)] -extern crate time; extern crate term; extern crate cargo; extern crate hamcrest; -- 2.30.2